-
Notifications
You must be signed in to change notification settings - Fork 387
feat: add languagewire as translation engine #1340
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds LanguageWire as a new translation engine. Updates package.json to include the engine enum value and new config keys for API key and API root. Extends Config with getters for languageWireApiKey and languageWireApiRoot. Introduces src/translators/engines/languagewire.ts implementing translate() via axios to POST to {apiRoot}/translations/text and transform responses. Registers the engine in src/translators/index.ts and exports LanguageWireTranslate. Sequence Diagram(s)sequenceDiagram
autonumber
participant U as Caller
participant T as Translator
participant LW as LanguageWireTranslate
participant API as LanguageWire API
U->>T: translate({ engine: "languagewire", text, from, to })
T->>LW: translate(options)
Note over LW: Resolve apiRoot (config/override), read apiKey
LW->>API: POST /translations/text<br/>headers: Bearer apiKey<br/>body: sourceText, targetLanguage,<br/>[sourceLanguage]
API-->>LW: 200 OK<br/>{ translation, detectedSourceLanguage }
LW->>LW: transform(response, options)
LW-->>T: TranslateResult
T-->>U: TranslateResult
alt Error
API-->>LW: 4xx/5xx error
LW-->>T: throw error
T-->>U: propagate error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (5)
src/core/Config.ts (1)
575-581: New LanguageWire config getters are correctly wired.The getters align with package.json keys (i18n-ally.translate.languagewire.apiKey/apiRoot). Consider normalizing apiRoot here (strip trailing slash) to keep callers simpler, but this is optional as the engine already handles it.
- static get languageWireApiRoot() { - return this.getConfig<string | null | undefined>('translate.languagewire.apiRoot') - } + static get languageWireApiRoot() { + const root = this.getConfig<string | null | undefined>('translate.languagewire.apiRoot') + return root ? root.replace(/\/$/, '') : root + }package.json (1)
1045-1047: Engine enum updated — verify documentation/UI strings show “LanguageWire”.The new engine value is added. Ensure any user-visible docs/help that list engines also mention languagewire.
src/translators/engines/languagewire.ts (3)
13-24: Only include sourceLanguage when explicitly provided (and not "auto").As written, undefined !== 'auto' evaluates true and spreads { sourceLanguage: undefined }, which is then serialized away but is brittle. Be explicit.
{ sourceText: options.text, reference: { source: 'USER', }, - targetLanguage: options.to, - // If source language is specified, include it - ...(options.from !== 'auto' && { sourceLanguage: options.from }), + targetLanguage: options.to, + // If source language is specified, include it + ...(options.from && options.from !== 'auto' ? { sourceLanguage: options.from } : {}), },
25-31: Add a request timeout and optionally set baseURL.Without a timeout, requests can hang indefinitely. A modest timeout improves resilience.
{ headers: { 'Content-Type': 'application/json', 'Accept': 'application/json', 'Authorization': `Bearer ${apiKey}`, }, + timeout: 15000, },
5-7: Optional: centralize apiRoot normalization.You could pre-normalize apiRoot in the class property to avoid repeating replace(//$/, '') elsewhere. Not a blocker.
export default class LanguageWireTranslate extends TranslateEngine { - apiRoot = 'https://lwt.languagewire.com/p/api/v1' + apiRoot = 'https://lwt.languagewire.com/p/api/v1' // no trailing slash
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
package.json(2 hunks)src/core/Config.ts(2 hunks)src/translators/engines/languagewire.ts(1 hunks)src/translators/index.ts(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/translators/index.ts (1)
src/translators/engines/languagewire.ts (1)
LanguageWireTranslate(5-56)
src/translators/engines/languagewire.ts (2)
src/translators/engines/base.ts (2)
TranslateOptions(6-10)TranslateResult(12-21)src/core/Config.ts (1)
Config(13-598)
🔇 Additional comments (4)
src/core/Config.ts (1)
183-185: LGTM on sortLocale getter.Non-functional formatting change only. No concerns.
package.json (1)
1109-1118: Verify i18n translation entries for new config descriptionsI wasn’t able to locate any VS Code NLS or locale JSON files containing the keys referenced by your new descriptions. Please confirm where your extension’s translation files live (e.g.
package.nls.json,package.nls.<lang>.json, or under alocales/folder) and ensure they include entries for:•
config.languagewire_api_key
•config.languagewire_api_rootFor example, in
package.nls.json:{ "config.languagewire_api_key": "LanguageWire API Key", "config.languagewire_api_root": "LanguageWire API Root URL" }src/translators/index.ts (1)
8-8: Engine registration and exports look correct.Import, engine map registration under 'languagewire', and re-export are consistent with other engines.
Also applies to: 18-19, 35-36
src/translators/engines/languagewire.ts (1)
8-34: Optional: map VS Code locales to LanguageWire expected codes.If LanguageWire expects MMT codes differing from BCP47, introduce a mapping (similar to other engines) to avoid 400s on uncommon locales. If LanguageWire accepts BCP47 as-is, ignore this.
Would you like me to add a minimal mapper (BCP47 -> MMT codes) after confirming LanguageWire’s expected codes?
| async translate(options: TranslateOptions) { | ||
| const apiKey = Config.languageWireApiKey | ||
| let apiRoot = this.apiRoot | ||
| if (Config.languageWireApiRoot) apiRoot = Config.languageWireApiRoot.replace(/\/$/, '') | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Guard against missing API key and undefined target language.
Calling the API without a key or target language will fail with a 401/validation error and yields a poor UX. Throw early with a clear message.
Apply:
async translate(options: TranslateOptions) {
const apiKey = Config.languageWireApiKey
let apiRoot = this.apiRoot
- if (Config.languageWireApiRoot) apiRoot = Config.languageWireApiRoot.replace(/\/$/, '')
+ if (Config.languageWireApiRoot) apiRoot = Config.languageWireApiRoot.replace(/\/$/, '')
+
+ if (!apiKey) {
+ throw new Error('LanguageWire: missing API key. Configure i18n-ally.translate.languagewire.apiKey in settings.')
+ }
+ if (!options.to) {
+ throw new Error('LanguageWire: target language "to" is required.')
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async translate(options: TranslateOptions) { | |
| const apiKey = Config.languageWireApiKey | |
| let apiRoot = this.apiRoot | |
| if (Config.languageWireApiRoot) apiRoot = Config.languageWireApiRoot.replace(/\/$/, '') | |
| async translate(options: TranslateOptions) { | |
| const apiKey = Config.languageWireApiKey | |
| let apiRoot = this.apiRoot | |
| if (Config.languageWireApiRoot) apiRoot = Config.languageWireApiRoot.replace(/\/$/, '') | |
| if (!apiKey) { | |
| throw new Error('LanguageWire: missing API key. Configure i18n-ally.translate.languagewire.apiKey in settings.') | |
| } | |
| if (!options.to) { | |
| throw new Error('LanguageWire: target language "to" is required.') | |
| } | |
| // …rest of implementation… | |
| } |
🤖 Prompt for AI Agents
In src/translators/engines/languagewire.ts around lines 8 to 12, add early
guards to validate Config.languageWireApiKey and the requested target language
before making the API call: if the API key is missing/empty throw a clear Error
like "LanguageWire API key is missing: set Config.languageWireApiKey", and if
the TranslateOptions does not include a target language (e.g.,
options.targetLanguage or options.target) throw "Missing target language in
translate options"; do these checks immediately after reading apiKey and
options, so the function fails fast with descriptive messages rather than
letting the request go to the API and return a 401/validation error.
| transform(response: any, options: TranslateOptions): TranslateResult { | ||
| const { text, from = 'auto', to = 'auto' } = options | ||
|
|
||
| // Get the translated text from the response | ||
| const translatedText = response.data.translation?.trim() | ||
|
|
||
| // Determine the detected source language | ||
| const detectedSource = response.data.detectedSourceLanguage?.mmtCode || from | ||
|
|
||
| const r: TranslateResult = { | ||
| text, | ||
| to, | ||
| from: detectedSource, | ||
| response, | ||
| result: translatedText ? [translatedText] : undefined, | ||
| linkToResult: '', | ||
| } | ||
|
|
||
| return r | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Harden transform against non-string translations and missing fields.
If the API returns a non-string translation, translation?.trim() will throw. Also, keep result consistently typed.
transform(response: any, options: TranslateOptions): TranslateResult {
const { text, from = 'auto', to = 'auto' } = options
- // Get the translated text from the response
- const translatedText = response.data.translation?.trim()
+ // Get the translated text from the response
+ const raw = response?.data?.translation
+ const translatedText = typeof raw === 'string' ? raw.trim() : undefined
- // Determine the detected source language
- const detectedSource = response.data.detectedSourceLanguage?.mmtCode || from
+ // Determine the detected source language
+ const detectedSource = response?.data?.detectedSourceLanguage?.mmtCode || from
const r: TranslateResult = {
text,
to,
from: detectedSource,
response,
result: translatedText ? [translatedText] : undefined,
linkToResult: '',
}
return r
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| transform(response: any, options: TranslateOptions): TranslateResult { | |
| const { text, from = 'auto', to = 'auto' } = options | |
| // Get the translated text from the response | |
| const translatedText = response.data.translation?.trim() | |
| // Determine the detected source language | |
| const detectedSource = response.data.detectedSourceLanguage?.mmtCode || from | |
| const r: TranslateResult = { | |
| text, | |
| to, | |
| from: detectedSource, | |
| response, | |
| result: translatedText ? [translatedText] : undefined, | |
| linkToResult: '', | |
| } | |
| return r | |
| } | |
| transform(response: any, options: TranslateOptions): TranslateResult { | |
| const { text, from = 'auto', to = 'auto' } = options | |
| // Get the translated text from the response | |
| const raw = response?.data?.translation | |
| const translatedText = typeof raw === 'string' ? raw.trim() : undefined | |
| // Determine the detected source language | |
| const detectedSource = response?.data?.detectedSourceLanguage?.mmtCode || from | |
| const r: TranslateResult = { | |
| text, | |
| to, | |
| from: detectedSource, | |
| response, | |
| result: translatedText ? [translatedText] : undefined, | |
| linkToResult: '', | |
| } | |
| return r | |
| } |
🤖 Prompt for AI Agents
src/translators/engines/languagewire.ts around lines 36 to 55, the transform()
assumes response.data.translation is a string and calls .trim(), which will
throw for non-strings or when fields are missing; also response.data may be
undefined and result should remain a string[] | undefined. Fix by defensively
reading nested fields (guard response && response.data), check typeof
translation === 'string' before trimming, otherwise coerce to
String(response.data.translation) only if not null/undefined or skip; set
translatedText to the trimmed string or undefined; ensure result is either an
array of strings when a valid translation exists or undefined; similarly guard
detectedSourceLanguage access and default to options.from.
No description provided.